Align params behavior for unlimited limits #713
Conversation
WalkthroughThe recent changes streamline limit checks and enhance documentation in the codebase. Notably, the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant LimitChecker
participant QueryProcessor
Client->>LimitChecker: Request query with maxSize and maxResultCount
LimitChecker->>LimitChecker: Check if values are nil or zero
LimitChecker->>QueryProcessor: Pass valid limits
QueryProcessor->>Client: Return query results
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
bdeneux
force-pushed
the
feat/params_check
branch
from
f9f0bdc
to
6e3888d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## main #713 +/- ##
==========================================
+ Coverage 54.42% 54.49% +0.06%
==========================================
Files 73 73
Lines 2901 2905 +4
==========================================
+ Hits 1579 1583 +4
Misses 1228 1228
Partials 94 94
|
bdeneux
force-pushed
the
feat/params_check
branch
from
6e3888d
to
c08f1e8
Compare
bdeneux
force-pushed
the
feat/params_check
branch
from
c08f1e8
to
6b474cb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (2)
docs/proto/logic.mdis excluded by!docs/proto/**/*.mdx/logic/types/params.pb.gois excluded by!**/*.pb.go
Files selected for processing (5)
- proto/logic/v1beta2/params.proto (1 hunks)
- x/logic/keeper/grpc_query_ask.go (1 hunks)
- x/logic/keeper/grpc_query_ask_test.go (10 hunks)
- x/logic/util/pointer.go (2 hunks)
- x/logic/util/pointer_test.go (2 hunks)
Files skipped from review due to trivial changes (1)
- proto/logic/v1beta2/params.proto
Additional comments not posted (8)
x/logic/util/pointer.go (1)
26-32: LGTM! Verify the function usage in the codebase.The new function
NonZeroOrDefaultUIntis well-defined and aligns with the existing utility functions.However, ensure that all function calls to
NonZeroOrDefaultUIntare correctly implemented in the codebase.x/logic/util/pointer_test.go (1)
59-86: LGTM!The new test function
TestNonZeroOrDefaultUIntis well-structured and covers various scenarios using a table-driven approach.x/logic/keeper/grpc_query_ask.go (1)
55-61: LGTM!The changes to the
checkLimitsfunction enhance the robustness of the limit checks by ensuring that the retrieved values forMaxSizeandMaxResultCountare non-zero before falling back to the default values.x/logic/keeper/grpc_query_ask_test.go (5)
129-140: LGTM!The new test case for
maxSizeset to zero is well-defined and ensures that the function handles queries correctly when there are no restrictions on the maximum size.
158-166: LGTM!The updated error messages for gas limits accurately reflect the changes in internal logic and provide clear information to developers.
334-352: LGTM!The new test case for
maxResultCountset to zero is well-defined and ensures that the function handles queries correctly when there are no restrictions on the maximum result count.
385-386: LGTM!The initialization of
maxResultCountandmaxSizeusingsdkmath.NewUintis consistent with the new data types and ensures correct initialization.
38-39: Verify the handling of the new data type.The data types of
maxResultCountandmaxSizehave been changed frominttouint64. Ensure that all related logic and function calls handle the new data type correctly.
Following this comment, this PR aligns the behavior for all parameters regarding the unlimited case. Most parameters are now configured to be considered unlimited if either a
nilvalue or a zero value is set. I chose to align the behavior of themax_sizeandmax_result_countparameters with this approach, as they previously did not follow the same behavior.Summary by CodeRabbit